Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: run integration tests outside LMS container #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

magajh
Copy link

@magajh magajh commented Sep 16, 2024

Overview

This PR introduces significant modifications to the GitHub Action for testing Open edX plugins in the Tutor environment. These changes are necessary for improving the effectiveness and reliability of our integration tests

Motivation for Changes

The initial implementation ran integration tests inside the LMS container of Tutor. This approach created several challenges:

  • Networking Issues: Running tests inside the same container restricted the ability to make requests to external hosts, leading to unreliable test outcomes.
  • Realistic Testing: To ensure that our integration tests reflect real-world interactions between our plugins and the Open edX platform, it is crucial to run them outside the Tutor container, maintaining a separation between the tests and the platform environment.

To address these issues, this PR modifies the workflow to run integration tests outside the Tutor container, improving the testing environment and results.

Workflow Steps and Their Necessity

  1. Set Tutor Environment Variables:

    • This step sets necessary environment variables like LMS_HOST, CMS_HOST, and paths for Tutor and plugins. These variables are essential for proper configuration and operation of Tutor.
  2. Install and Prepare Tutor:

    • Installs the specified version of Tutor, configures it with the necessary host settings, and launches Tutor in non-interactive mode. This ensures that the Tutor environment is ready for testing.
  3. Configure Caddyfile and Open edX Settings:

    • Creates a patches.yml file to customize the Caddyfile and Open edX production settings. This configuration allows for the necessary routing for testing with multiple sites and security settings for our tests.
  4. Install Plugin and Extra Requirements:

    • Installs the specified plugin and any additional dependencies required for testing. This ensures that the environment is properly set up for the plugin being tested.
  5. Run Migrations:

    • Executes database migrations for both the LMS and CMS. This step is crucial to ensure that the database schema is up-to-date with the changes introduced by the plugins and other dependencies.
  6. Load Initial Data for the Tests:

    • Optionally copies fixtures into the LMS container and loads them into the database. This allows tests to run with pre-defined data, ensuring consistency and reliability.
  7. Curl Heartbeat:

    • Verifies that the LMS is running and responsive by checking the heartbeat endpoint. This step helps ensure that the environment is fully operational before running tests.
  8. Run Integration Tests:

    • Executes the specified shell script to run the integration tests. This step is where the actual testing takes place, utilizing the environment configured in previous steps.

Personalization Options

This workflow includes several inputs that can be customized:

  • app_name: The name of the application to test (e.g., eox-core).
  • tutor_version: The version of Tutor to use, allowing for testing against different Tutor releases.
  • shell_file_to_run: The path to the shell script that contains the integration tests.
  • openedx_extra_pip_requeriments: Optional additional dependencies needed for testing.
  • fixtures_file: Optional path to a JSON file containing initial data to load for the tests.

Testing the Workflow

Steps to test this workflow:

  1. You can create a branch from any of the EOX plugins. For example, create a branch from bav/add-integration-tests in eox-core. I’m using this branch as an example because it includes several integration tests.

  2. Add the script that installs the requirements and runs the tests, for example:

    #!/bin/bash
    
    bgreen () { printf "\n\n\e[1m\e[32m" ; $@ ; printf "\e[0m\n\n"; }
    
    bgreen echo "Installing test requirements"
    make install-dev-dependencies
    
    bgreen echo "Running integration tests"
    pytest eox_core/api/v1/tests/integration/test_views.py
  3. Ensure that the workflow calls the action with the corresponding inputs and uses the branch mjh/run-integration-tests-outside-container. Here’s an example of the file that executes the workflow:

    name: Tutor Integration Tests
    on: [pull_request]
    
    jobs:
      integration-test:
        name: Tutor Integration Tests
        runs-on: ubuntu-latest
        strategy:
          matrix:
            tutor_version: ['<18.0.0', '<19.0.0']
        steps:
          - name: Run Integration Tests
            uses: eduNEXT/integration-test-in-tutor@mjh/run-integration-tests-outside-container
            with:
              tutor_version: ${{ matrix.tutor_version }}
              app_name: 'eox-core'
              openedx_extra_pip_requirements: 'eox-tenant'
              shell_file_to_run: 'scripts/integration.sh'
              fixtures_file: 'fixtures/initial_data.json'
  4. Check that on-push, the integration tests run successfully, as shown in this test branch: test: add integration tests eox-core#278


⭐ Important:

This PR needs to be updated with the implementation from PR #3 to support running the tests in Tutor nightly. The update will be made once PR #3 is merged.

@magajh magajh changed the title Mjh/run integration tests outside container refactor: run integration tests outside LMS container Sep 16, 2024
@magajh magajh force-pushed the mjh/run-integration-tests-outside-container branch 7 times, most recently from 43ba8c0 to 89cac69 Compare September 17, 2024 00:04
@magajh magajh force-pushed the mjh/run-integration-tests-outside-container branch from 60e82aa to a016821 Compare September 17, 2024 12:46
@magajh magajh requested a review from a team September 17, 2024 12:48
@magajh
Copy link
Author

magajh commented Sep 17, 2024

Note to reviewers: I'm working on updating the README

Copy link

@BryanttV BryanttV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magajh, This looks great! thank you!

Just to note, when adding these changes we must modify all eox's that use this action, as the commands in your scripts will not work.

description: "Optional extra pip requirements to install in Open edX. E.g: 'package1==1.0 package2>=2.0'"
required: false
default: ""
python_version:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which cases would this input (python_version) be useful?

Comment on lines +89 to +92
echo "Copying fixtures file to the LMS container"
tutor local exec lms mkdir -p /openedx/fixtures
tutor local dc cp ${{ inputs.fixtures_file }} lms:/openedx/fixtures/fixture.json
tutor local exec lms python manage.py lms loaddata /openedx/fixtures/fixture.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is copying better than mounting the file directly with tutor mounts ... ?

Comment on lines +112 to +115
if [ ! -d ".venv" ]; then
echo "Virtual environment creation failed"
exit 1
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has the venv creation failed before? I can't think of why it could fail, but there may be edge cases I haven't considered.

Also, should we create the environment before installing any dependencies like tutor?

Comment on lines +55 to +70
cat << 'EOF' > plugins/patches.yml
name: patches
patches:
caddyfile: |
{$default_site_port} {
import proxy "lms:8000"
}
openedx-cms-production-settings: |
ALLOWED_HOSTS = ["*"]
ALLOWED_AUTH_APPLICATIONS = ['cms-sso', 'cms-sso-dev']
openedx-lms-production-settings: |
ALLOWED_HOSTS = ["*"]
ALLOWED_AUTH_APPLICATIONS = ['cms-sso', 'cms-sso-dev']
EOF
tutor plugins enable patches
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this file to a Caddyfile outside the action definition, or would that be overkill? Composite actions can access their files, so maybe we can leverage that, although it might be more complex for little gain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants